[박연희] sprint1 #21
[박연희] sprint1 #21
Conversation
addiescode-sj
left a comment
addiescode-sj
left a comment
There was a problem hiding this comment.
수고하셨어요!
몇가지 개선점만 다듬어보면 멋진 결과물이 나오겠네요 :)
주요 리뷰 포인트
- 폰트 최적화
- 이미지 alt 속성 추가
- 미디어 쿼리 사용 관련 제안
- 네이밍 컨벤션, 네이밍 케이스
| <div class="logo"> | ||
| <a href="/"> | ||
| <img class="logo-img" src="images/img_logo.png" alt="img_logo"> | ||
| <img class="logo-mobile" src="images/img_logo_m.png" alt="img_logo_m"> | ||
| </a> |
There was a problem hiding this comment.
오호 모바일 반응형에서는 판다 로고를 없애고 보이게끔 해주셨군요? 디테일을 볼줄 아시네요 👍
|
|
||
| <div class="item"> | ||
| <h1>일상의 모든 물건을 거래해 보세요</h1> | ||
| <a class="btn btn_large btn-color1" href="/items">구경하러 가기</a> |
There was a problem hiding this comment.
접근성을 고려해 <a href="/" aria-label="홈으로 이동"> 과 같이 aria-label 혹은 role을 추가해볼까요?
| <h1>일상의 모든 물건을 거래해 보세요</h1> | ||
| <a class="btn btn_large btn-color1" href="/items">구경하러 가기</a> | ||
| </div> | ||
| <img src="images/Img_home_top.png" alt="Img_home_top"> |
There was a problem hiding this comment.
귀찮더라도 항상 img태그에는 이미지를 최대한 구체적으로 잘 설명해주는 alt 속성을 추가해주세요.
웹 접근성뿐만 아니라, 이미지 로딩 실패 시 대체 텍스트를 표시하거나 검색엔진최적화에도 도움을 줄 수 있기 때문에 꼭 사용해주시는게 좋습니다.
레퍼런스로 애플 공식 웹사이트에 가서 alt텍스트를 어떻게 사용했는지 참고해보시면:
<img src="/kr/macbook-air/images/overview/design/design_hero_static__e56c1v71mr6u_large.jpg" onload="__lp(event)" alt="열려있는 MacBook Air 13 및 15의 모습. 한 대에는 디자인 작업을 진행 중인 화면이, 다른 한 대에는 이메일과 스프레드시트를 넘나들며 멀티태스킹을 하는 화면이 표시되어 있습니다">이런식으로 alt 속성에 이미지 설명을 위해 구체적이고 명확한 설명을 제공하는 모습을 확인해보실 수 있습니다 :)
| </section> | ||
| <section class="content-section"> | ||
| <div class="content-box"> | ||
| <img src="images/Img_home_03.png" alt=""> |
| <a href="/facebook" target="_blank"><img src="images/ic_facebook.png" alt="facebook"></a> | ||
| <a href="/twitter" target="_blank"><img src="images/ic_twitter.png" alt="twitter"></a> | ||
| <a href="/youtube" target="_blank"><img src="images/ic_youtube.png" alt="youtube"></a> | ||
| <a href="/instagram" target="_blank"><img src="images/ic_instagram.png" alt="instagram"></a> |
There was a problem hiding this comment.
외부 링크의 경우, 보안을 위해 rel="noopener noreferrer"를 추가해주세요! :)
noopener: 새 탭에서 열린 페이지가 원본 페이지에 접근하는 것을 방지
noreferrer: 리퍼러 정보가 새 페이지로 전달되는 것을 방지
| .logo-mobile { | ||
| display: none; | ||
| } |
There was a problem hiding this comment.
개인의 선택이긴 한데, 미디어쿼리를 스타일 블록마다 작성해주는 방식이 있고 지금처럼 한꺼번에 아래에서 작성해주는 방식이 있는데, 저는 전자를 선호하긴합니다 ㅎㅎ 이런 방식도 있다는걸 참고해두시면 좋을것같아요 :)
| .logo-mobile { | |
| display: none; | |
| } | |
| .logo-mobile { | |
| display: none; | |
| @media (max-width: 375px) { | |
| display: block; | |
| } | |
| } |
| display: flex; | ||
| justify-content: center; | ||
| align-items: center; |
There was a problem hiding this comment.
이런 스타일 블록들도 여러 군데에서 반복되고있다면 따로 유틸용 클래스를 만들어서 재사용하시면 중복을 줄일 수 있겠죠?
e.g
.flex-center {
display: flex;
justify-content: center;
align-items: center;
}|
|
||
| /* main > banner-section */ | ||
| .banner-section { | ||
| background-color: #CFE5FF; |
There was a problem hiding this comment.
자주 사용되는 값들은 css 변수로 만드시면 편하실거예요!
예시)
:root {
/* Colors */
--color-primary: #3692ff;
--color-primary-dark: #1967d6;
--color-text: #374151;
--color-text-light: #9ca3af;
--color-background: #ffffff;
--color-background-alt: #fcfcfc;
--color-background-landing: #cfe5ff;
--color-background-footer: #111827;
--color-text-light: #f3f4f6;
--color-text-lighter: #f9fafb;
/* Typography */
--font-family-base: pretendard, sans-serif;
--font-family-heading: "ROKAFSansMedium", sans-serif;
/* Spacing */
--spacing-xs: 8px;
--spacing-sm: 12px;
--spacing-md: 24px;
--spacing-lg: 32px;
--spacing-xl: 64px;
--spacing-xxl: 138px;
/* Layout */
--container-max-width: 1520px;
--container-content-width: 1120px;
--container-section-width: 988px;
/* Border Radius */
--border-radius-sm: 8px;
--border-radius-lg: 40px;
}| gap: 20px; | ||
| padding-top: 32px; | ||
| } | ||
| footer .copyRight { |
There was a problem hiding this comment.
여기서는 갑자기 카멜케이스를 쓰셨네요! 네이밍 케이스, 네이밍 컨벤션 잘 맞춰볼까요?
두개 참고해보시고, 일관적인 네이밍 규칙을 적용해보시면 좋을 것 같습니다!
| } | ||
|
|
||
| /* 반응형 */ | ||
| @media (max-width: 1950px) { |
There was a problem hiding this comment.
요렇게 하시는것보다는 작은 화면에서부터 미디어 쿼리에 의한 재정의를 하는 구조로 바꿔보시는게 불필요한 스타일 오버라이드를 제거하는데 좀 더 유리합니다.
그리고 미디어 쿼리를 사용하실때 공통으로 사용되는 값들을 css 변수로 정의한다음 각 브레이크 포인트에서 변수값만 변경해주는식으로 활용하시면 스타일도 중복 제거하면서 유지보수에도 용이해집니다.
코멘트 참고해보시고 리팩토링해보세요! :)
질문에 대한 답변
심화 미션을 구현하셨군요! 기본 요구사항을 따르면 여백은 항상 양옆 200px을 유지하라고되어있지만, 모바일 등 스크린 사이즈가 작아질경우 여백값을 변경되게끔하는게 의도였다면 괜찮습니다! |
2 participants
요구사항
기본
심화
주요 변경사항
스크린샷 & 배포 사이트
멘토에게
브라우저의 넓이 크기가 작아질 경우 상단 로고와 로그인 버튼 여백이 너무 큰 거 같아서 브라우저 창의 크기가 작아지면 여백 값도 변경되게 CSS를 추가 했는데
기본 요구사항을 어긴 걸까요? 맞는 걸까요?